Skip to content

[NAE-2440] Fix PluginActionApi bean creation in Spring context#443

Merged
machacjozef merged 3 commits into
release/7.0.0-rev10from
NAE-2440
May 25, 2026
Merged

[NAE-2440] Fix PluginActionApi bean creation in Spring context#443
machacjozef merged 3 commits into
release/7.0.0-rev10from
NAE-2440

Conversation

@renczesstefan
Copy link
Copy Markdown
Member

@renczesstefan renczesstefan commented May 25, 2026

Description

Fixes NAE-2440

Dependencies

No new dependencies were introduced

Third party dependencies

  • No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

This was tested manually and with unit tests.

Test Configuration

Name Tested on
OS macOS Tahoe 26.3
Runtime Java 21
Dependency Manager Maven 3.9.9n
Framework version Spring Boot 3.4.4
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @machacjozef
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Summary by CodeRabbit

New Features

  • Query process availability status (up, down, or not found) for single or multiple process identifiers
  • Batch retrieve availability information for multiple processes in a single request
  • Access system user authentication principal details in alternative format

Review Change Stack

Added methods to Action API for checking the availability of processes (`isProcessUp`, `isProcessDown`) and retrieving availability statuses (`getProcessAvailability`). Introduced `ProcessAvailability` and `ProcessAvailabilities` classes for streamlined handling of process states. Updated `ActionApiImpl` to integrate `IPetriNetService` for process status checks.
@renczesstefan renczesstefan self-assigned this May 25, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@renczesstefan, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 33 minutes and 55 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 17bd186f-4373-407c-a7ca-7481110e5de8

📥 Commits

Reviewing files that changed from the base of the PR and between eb098e1 and 11e4836.

📒 Files selected for processing (5)
  • application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiConfiguration.java
  • application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailabilities.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailability.java

Walkthrough

This PR adds process availability query capabilities to the ActionApi. New data types ProcessAvailability and ProcessAvailabilities represent process status (UP, DOWN, NOT_FOUND). The ActionApi interface gains isProcessUp, isProcessDown, and three getProcessAvailability overloads (single, list, varargs). ActionApiImpl implements these by injecting IPetriNetService, querying PetriNet for process existence, and returning typed availability results. A getSystemUserDto() method is also added.

Changes

Process Availability Query API

Layer / File(s) Summary
ProcessAvailability data types
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailability.java, ProcessAvailabilities.java
ProcessAvailability record holds a process identifier and Status (UP, DOWN, NOT_FOUND) enum with factory methods and predicates. ProcessAvailabilities wraps a list and provides per-process and aggregate status checks via stream matching.
ActionApi interface contract
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
Adds getSystemUserDto() returning AuthPrincipalDto, plus isProcessUp, isProcessDown, and three getProcessAvailability overloads accepting single identifier, List<String>, or varargs.
ActionApiImpl implementation
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Injects IPetriNetService and implements all new interface methods by querying PetriNet for process presence, converting to ProcessAvailability/ProcessAvailabilities, and providing system user DTO from existing system user.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant ActionApiImpl
  participant IPetriNetService
  participant PetriNet
  
  Caller->>ActionApiImpl: getProcessAvailability(identifier)
  ActionApiImpl->>IPetriNetService: getDefaultVersionByProcessIdentifier(identifier)
  IPetriNetService->>PetriNet: lookup by identifier
  PetriNet-->>IPetriNetService: PetriNet or null
  IPetriNetService-->>ActionApiImpl: result
  ActionApiImpl->>ActionApiImpl: ProcessAvailability.from(identifier, isPresent)
  ActionApiImpl-->>Caller: ProcessAvailability
  
  Caller->>ActionApiImpl: getProcessAvailability(List)
  ActionApiImpl->>IPetriNetService: query each identifier
  IPetriNetService-->>ActionApiImpl: multiple results
  ActionApiImpl->>ActionApiImpl: stream to ProcessAvailability list
  ActionApiImpl-->>Caller: ProcessAvailabilities
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

bugfix, breaking change, Medium

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title references 'PluginActionApi bean creation' but the changes actually focus on adding process availability/status checking methods (isProcessUp, isProcessDown, getProcessAvailability) to ActionApi and ActionApiImpl, plus new ProcessAvailability/ProcessAvailabilities classes. Consider retitling to reflect the main changes, such as 'NAE-2440 Add process availability methods to ActionApi' or similar, as the current title does not accurately describe the primary additions in this changeset.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.23% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added bugfix A change that fixes a bug breaking change Fix or feature that would cause existing functionality doesn't work as expected Medium labels May 25, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java`:
- Around line 365-376: The batch methods getProcessAvailability(List<String>)
and getProcessAvailability(String...) currently stream over inputs and will NPE
on a null argument; add null checks at the start of each method to handle null
input gracefully (e.g., treat a null List or null varargs as an empty collection
and return new ProcessAvailabilities(Collections.emptyList()) or proceed with an
empty stream), and also defensively filter out any null elements before mapping
(e.g., filter(Objects::nonNull)) so mapping via this::getProcessAvailability
never receives null.
- Around line 359-362: The current getProcessAvailability method calls
petriNetService.getDefaultVersionByIdentifier and passes petriNet != null to
ProcessAvailability.from, which never yields Status.NOT_FOUND; instead detect
the missing process explicitly: call
petriNetService.getDefaultVersionByIdentifier inside a try/catch (or check for
null if the service returns null), and when the PetriNet is absent or a "not
found" exception occurs return ProcessAvailability.from(processIdentifier,
Status.NOT_FOUND); otherwise return ProcessAvailability.from(processIdentifier,
Status.UP) (or keep existing successful mapping). Update getProcessAvailability,
reference petriNetService.getDefaultVersionByIdentifier,
ProcessAvailability.from and Status.NOT_FOUND/Status.UP accordingly.

In
`@nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailabilities.java`:
- Line 3: In ProcessAvailabilities, make the process identifier comparison
null-safe by replacing any direct .equals calls on
processAvailability.processIdentifier() with
Objects.equals(processAvailability.processIdentifier(), targetIdentifier) (or
equivalent null-safe comparison), and add the necessary import for
java.util.Objects; update all occurrences of the equality check (the comparisons
around processAvailability.processIdentifier() in the lookup logic) to use
Objects.equals to avoid NullPointerException.
- Around line 106-107: The isAll(ProcessAvailability.Status status) helper
currently uses availabilities.stream().allMatch(...) which is vacuously true for
an empty availabilities list; update isAll to also require the collection is
non-empty (e.g. !availabilities.isEmpty() &&
availabilities.stream().allMatch(...)) so that isAllUp(), isAllDown(), and
isAllNotFound() return false when there are no process availabilities; reference
the isAll method, availabilities field, and ProcessAvailability.Status in your
change.
- Line 11: The record ProcessAvailabilities currently stores the caller-provided
mutable List reference; change its canonical constructor to defensively copy
and/or wrap the incoming list (e.g., assign List.copyOf(availabilities) or an
unmodifiable copy) so the record field is immutable and the internal state can't
be mutated by callers; update the record's constructor/body to perform the copy
and optionally validate non-null input in the ProcessAvailabilities(List
availabilities) constructor.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 42712dfe-55ca-4eeb-8afe-ff842ff8075c

📥 Commits

Reviewing files that changed from the base of the PR and between eb098e1 and 4319cc8.

📒 Files selected for processing (4)
  • application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailabilities.java
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ProcessAvailability.java

machacjozef
machacjozef previously approved these changes May 25, 2026
…dling

Added null checks for `processIdentifiers` in `getProcessAvailability` overloads to prevent potential null pointer exceptions. Refactored `ProcessAvailabilities` to enforce immutability and improve consistency when handling availability lists.
@renczesstefan renczesstefan requested a review from machacjozef May 25, 2026 14:22
@renczesstefan
Copy link
Copy Markdown
Member Author

@coderabbitai please make review once more, I already made changes according to your commnts.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

@renczesstefan Great progress — all five previously flagged issues have been addressed. Here is the updated review:


✅ Previously raised issues — all resolved

# File Issue Status
1 ActionApiImpl.java petriNet != null made NOT_FOUND unreachable ✅ Fixed
2 ActionApiImpl.java No null-guard on batch methods ✅ Fixed
3 ProcessAvailabilities.java Non-null-safe .equals() on processIdentifier() ✅ Fixed
4 ProcessAvailabilities.java isAll() vacuously true on empty list ✅ Fixed
5 ProcessAvailabilities.java Mutable list stored in record ✅ Fixed

🔍 Remaining minor observations

1. Indentation inconsistency in ActionApiImpl.getProcessAvailability(String) (minor)

// current (inconsistent indentation inside the if-block)
if (petriNet == null) {
        return ProcessAvailability.notFound(processIdentifier);   // 12 spaces
    }                                                              // 8 spaces

Should be:

if (petriNet == null) {
    return ProcessAvailability.notFound(processIdentifier);
}

2. Null elements inside batch collections are not filtered (low severity)

The original comment asked to also add .filter(Objects::nonNull) before .map(...) in both batch overloads. If a caller passes a list containing null strings, the call cascades to petriNetService.getDefaultVersionByIdentifier(null), which may throw. Consider adding:

public ProcessAvailabilities getProcessAvailability(List<String> processIdentifiers) {
    Objects.requireNonNull(processIdentifiers, "processIdentifiers cannot be null");
    return new ProcessAvailabilities(processIdentifiers.stream()
            .filter(Objects::nonNull)
            .map(this::getProcessAvailability)
            .collect(Collectors.toList()));
}

3. Redundant availabilities != null check in isAll() (trivial)

Since the canonical constructor now uses List.copyOf(availabilities), which throws NullPointerException if given null, the field availabilities can never be null at runtime. The availabilities != null && guard in isAll() is now dead code. It's harmless but could confuse future readers.

// can safely be simplified to:
private boolean isAll(ProcessAvailability.Status status) {
    return !availabilities.isEmpty() && availabilities.stream().allMatch(pa -> pa.status() == status);
}

4. No unit tests visible in the repository

The PR description states tests were performed, but no test file (e.g., ProcessAvailabilityTest or updated ActionApiImplTest) was found in the repository. If tests exist but are in a separate module or not yet committed, please ensure they are included before merging.


Overall the code quality has improved significantly. The remaining items are minor polish. The most important one to consider is the null-element filtering in the batch methods (item 2).

✅ Actions performed

Full review triggered.

@machacjozef machacjozef merged commit 837a70b into release/7.0.0-rev10 May 25, 2026
6 of 7 checks passed
@machacjozef machacjozef deleted the NAE-2440 branch May 25, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Fix or feature that would cause existing functionality doesn't work as expected bugfix A change that fixes a bug Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants